-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ack & timeout relay msgs #1810
Ack & timeout relay msgs #1810
Conversation
|
const PacketRows = ({ | ||
packet, | ||
dataParser, | ||
}: { | ||
packet: Packet; | ||
dataParser?: (arg: Uint8Array) => ReactElement; | ||
}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because a number of messages need to display Packet
data, abstracted it here
@@ -140,5 +232,23 @@ export const IbcRelayComponent = ({ value }: { value: IbcRelay }) => { | |||
return <UpdateClientComponent update={update} />; | |||
} | |||
|
|||
if (value.rawAction?.is(MsgTimeout.typeName)) { | |||
const timeout = new MsgTimeout(); | |||
value.rawAction.unpackTo(timeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: since the proto definition of rawAction
is Any
, could there potentially be errors when unpacking? It might be a good idea to wrap the unpackTo method in a try-catch block when deserializing the rawAction into a message type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the conditional block it's in, it should unbpack fine. However, we want this to be fault-tolerant for unexpected values or deserialization errors. Will wrap in try/catch.
return ( | ||
<> | ||
<ParsedPacketData data={packet.data} dataParser={dataParser} /> | ||
{!!packet.sequence && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment: what does a packet sequence number of 0 represent? and if it's valid, wouldn't that be treated as falsy? are there other such instances like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, will change any field that returns a primitive (required field) to show without a conditional check.
return ( | ||
<> | ||
<ParsedPacketData data={packet.data} dataParser={dataParser} /> | ||
{!!packet.sequence && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, will change any field that returns a primitive (required field) to show without a conditional check.
@@ -140,5 +232,23 @@ export const IbcRelayComponent = ({ value }: { value: IbcRelay }) => { | |||
return <UpdateClientComponent update={update} />; | |||
} | |||
|
|||
if (value.rawAction?.is(MsgTimeout.typeName)) { | |||
const timeout = new MsgTimeout(); | |||
value.rawAction.unpackTo(timeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the conditional block it's in, it should unbpack fine. However, we want this to be fault-tolerant for unexpected values or deserialization errors. Will wrap in try/catch.
}; | ||
|
||
// Packet data stored as json string encoded into bytes | ||
const parseRecvPacket = (packetData: Uint8Array) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was informed that all of these messages expect to have a FungibleTokenPacketData
inside. Will refactor a bit for that case.
4dc1cdf
to
c40814d
Compare
A follow up from: #1805
Able to display IBC relay acknowledgements & timeouts in the transaction details page: